String #167

dinosaure · 2024-03-22T10:18:44Z

On top of #166

hannesm · 2024-04-04T21:50:23Z

The ecdsa_sig stuff is a bit horrible.. I'm unsure whether we should go through Z instead? The downside is that we would construct a Z just to validate (non-negative) and deconstruct it again -- and on the encoding site, just to add or remove some 0x00...

What needs to be done for encoding:

if the signature has the high bit set of the first byte, we need to prepend a 0x00 byte (to avoid that being treat as a negative number)
if the signature has leading 0x00 bytes, they should be removed (unless the next byte is > 0x7F, where again we need a 0x00 in front) <- this may happen if the signature simply has some leading 0s...

For decoding, the asn.1 integer already takes care of avoiding the redundant form (no superfluous leading 0x00 bytes) -- all we need to check that nobody gave us a negative number (thus first byte must be < 0x80). We cut away a potentially leading 0 since mirage-crypto-ec doesn't like these potentially too long (in terms of too many bytes, although they are 0) values...

hannesm · 2024-04-08T08:31:47Z

Still issues on 32bit architectures and ppc64... need to investigate, will do with some local system to test with.

Cstruct.create does this. If we don't initialize bytes with '\000', Field_element.zero can be something else than '\000'. It's a fix for mirleft/ocaml-x509#167.

dinosaure · 2024-04-20T12:42:28Z

Tested locally with docker and mirage/mirage-crypto#226 and it's work 🎉.

Cstruct.create does this. If we don't initialize bytes with '\000', Field_element.zero can be something else than '\000'. It's a fix for mirleft/ocaml-x509#167. Co-authored-by: Hannes Mehnert <[email protected]>

hannesm · 2024-06-11T08:23:58Z

The last commit represents a serial number to be an unsigned integer with at most 20 octets. This is a breaking change, and makes the behaviour more strict in respect to RFC 5280. Taking into consideration that e.g. the NSS trust anchors serials are fine, my plan is to document and release, and if there's pushback we will revise the checks.

@hannesm

CHANGES: ### Breaking changes * mirage-crypto: Poly1305 API now uses string (mirage/mirage-crypto#203 @hannesm) * mirage-crypto: Poly1305 no longer has type alias "type mac = string" (mirage/mirage-crypto#232 @hannesm) * mirage-crypto: the API uses string instead of cstruct (mirage/mirage-crypto#214 @reynir @hannesm) * mirage-crypto: Hash module has been removed. Use digestif if you need hash functions (mirage/mirage-crypto#213 @hannesm) * mirage-crypto: the Cipher_block and Cipher_stream modules have been removed, its contents is inlined: Mirage_crypto.Cipher_block.S -> Mirage_crypto.Block Mirage_crypto.Cipher_stream.S -> Mirage_crypto.Stream Mirage_crypto.Cipher_block.AES.CTR -> Mirage_crypto.AES.CTR (mirage/mirage-crypto#225 @hannesm, suggested in mirage/mirage-crypto#224 by @reynir) * mirage-crypto-pk: s-expression conversions for private and public keys (Dh, Dsa, Rsa) have been removed. You can use PKCS8 for encoding and decoding `X509.{Private,Public}_key.{en,de}code_{der,pem}` (mirage/mirage-crypto#208 @hannesm) * mirage-crypto-pk: in the API, Cstruct.t is no longer present. Instead, string is used (mirage/mirage-crypto#211 @reynir @hannesm) * mirage-crypto-rng: the API uses string instead of Cstruct.t. A new function `generate_into : ?g -> bytes -> ?off:int -> int -> unit` is provided (mirage/mirage-crypto#212 @hannesm @reynir) * mirage-crypto-ec: remove NIST P224 support (mirage/mirage-crypto#209 @hannesm @Firobe) * mirage-crypto: in Uncommon.xor_into the arguments ~src_off and ~dst_off are required now (mirage/mirage-crypto#232 @hannesm), renamed to unsafe_xor_into (98f01b14f5ebf98ba0e7e9c2ba97ec518f90fddc) * mirage-crypto-pk, mirage-crypto-rng: remove type alias "type bits = int" (mirage/mirage-crypto#236 @hannesm) ### Bugfixes * mirage-crypto (32 bit systems): CCM with long adata (mirage/mirage-crypto#207 @reynir) * mirage-crypto-ec: fix K_gen for bitlen mod 8 != 0 (reported in mirage/mirage-crypto#105 that P521 test vectors don't pass, re-reported mirage/mirage-crypto#228, fixed mirage/mirage-crypto#230 @Firobe) * mirage-crypto-ec: zero out bytes allocated for Field_element.zero (reported mirleft/ocaml-x509#167, fixed mirage/mirage-crypto#226 @dinosaure) ### Data race free * mirage-crypto (3DES): avoid global state in key derivation (mirage/mirage-crypto#223 @hannesm) * mirage-crypto-rng: use atomic instead of reference to be domain-safe (mirage/mirage-crypto#221 @dinosaure @reynir @hannesm) * mirage-crypto, mirage-crypto-rng, mirage-crypto-pk, mirage-crypto-ec: avoid global buffers, use freshly allocated strings/bytes instead, avoids data races (mirage/mirage-crypto#186 mirage/mirage-crypto#219 @dinosaure @reynir @hannesm) ### Other changes * mirage-crypto: add {de,en}crypt_into functions (and unsafe variants) to allow less buffer allocations (mirage/mirage-crypto#231 @hannesm) * mirage-crypto-rng-miou: new package which adds rng support with miou (mirage/mirage-crypto#227 @dinosaure) * PERFORMANCE mirage-crypto: ChaCha20/Poly1305 use string instead of Cstruct.t, ChaCha20 interface unchanged, performance improvement roughly 2x (mirage/mirage-crypto#203 @hannesm @reynir) * mirage-crypto-ec, mirage-crypto-pk, mirage-crypto-rng: use digestif for hashes (mirage/mirage-crypto#212 mirage/mirage-crypto#215 @reynir @hannesm) * mirage-crypto-rng: use a set for entropy sources instead of a list (mirage/mirage-crypto#218 @hannesm) * mirage-crypto-rng-mirage: provide a module type S (for use instead of mirage-random in mirage) (mirage/mirage-crypto#234 @hannesm)

x509.opam

hannesm · 2024-07-17T10:07:12Z

I removed the pin depends. This looks good to go, all we need is a changes entry, and a decision whether to add #164 to the release (it'll be a 1.0, I'm happy to include #164).

I'm fine to merge, I can as well tag a release on Github once we're good. Since I don't have my laptop this month, I won't be able to do a dune-release / opam-publish (or PR to opam-repository), but I'm sure once a release is here someone (@dinosaure ?) can run opam-publish to get something onto opam-repository.

dinosaure · 2024-07-17T12:44:17Z

Yes, if the tag is available and the tarball, I will be happy to make a release into opam-repository then 👍

CHANGES.md

hannesm · 2024-07-17T21:35:16Z

lib/x509.mli

-  val decode_pkcs1_digest_info : Cstruct.t ->
-    (Mirage_crypto.Hash.hash * Cstruct.t, [> `Msg of string ]) result
+  val decode_pkcs1_digest_info : string ->
+    ([ `MD5 | `SHA1 | `SHA224 | `SHA256 | `SHA384 | `SHA512 ] * string, [> `Msg of string ]) result


looking at this, I'm unsure. sometimes Digestif.hash' is used, but here and below it is explicit. Is this intentional (sorry I lost context and am on a mobile phone screen)

Digestif.hash' has more variants that this one. On the interface level, if we use Digestif.hash', this would mean we'd have to handle cases like RIPEMD160, for example, in the implementation. To avoid unnecessary cases, this is why we make variants explicit.

hannesm · 2024-07-17T21:36:00Z

lib/x509.mli

-    | `Bad_encoding of Distinguished_name.t * string * Cstruct.t
-    | `Hash_not_allowed of Distinguished_name.t * Mirage_crypto.Hash.hash
+    | `Bad_encoding of Distinguished_name.t * string * string
+    | `Hash_not_allowed of Distinguished_name.t * [ `MD5 | `SHA1 | `SHA224 | `SHA256 | `SHA384 | `SHA512 ]


same as above

hannesm · 2024-07-17T21:37:05Z

lib/x509.mli

@@ -1078,11 +1076,11 @@ module OCSP : sig
  type cert_id

  (** [create_cert_id issuer serial] creates cert_id for this serial *)
-  val create_cert_id : ?hash:Mirage_crypto.Hash.hash -> Certificate.t -> Z.t ->
+  val create_cert_id : ?hash:[ `MD5 | `SHA1 | `SHA224 | `SHA256 | `SHA384 | `SHA512 ] -> Certificate.t -> string ->


and again, explicit hashes instead of Digestif.hash'

CHANGES.md

hannesm · 2024-07-18T10:28:38Z

thanks a lot

@reynir

What's Changed - Fix mixup of subject and hash in error message by @reynir in mirleft/ocaml-x509#165 - Use string instead of cstruct by @dinosaure in mirleft/ocaml-x509#167 - Rename Authenticator.server_{cert,key}_fingerprint by @reynir in mirleft/ocaml-x509#164 - Add Certificate.fold_decode_pem_multiple by @art-w in mirleft/ocaml-x509#169 New Contributors - @art-w made their first contribution in mirleft/ocaml-x509#169

@hannesm

CHANGES: ### Breaking changes * mirage-crypto: Poly1305 API now uses string (mirage/mirage-crypto#203 @hannesm) * mirage-crypto: Poly1305 no longer has type alias "type mac = string" (mirage/mirage-crypto#232 @hannesm) * mirage-crypto: the API uses string instead of cstruct (mirage/mirage-crypto#214 @reynir @hannesm) * mirage-crypto: Hash module has been removed. Use digestif if you need hash functions (mirage/mirage-crypto#213 @hannesm) * mirage-crypto: the Cipher_block and Cipher_stream modules have been removed, its contents is inlined: Mirage_crypto.Cipher_block.S -> Mirage_crypto.Block Mirage_crypto.Cipher_stream.S -> Mirage_crypto.Stream Mirage_crypto.Cipher_block.AES.CTR -> Mirage_crypto.AES.CTR (mirage/mirage-crypto#225 @hannesm, suggested in mirage/mirage-crypto#224 by @reynir) * mirage-crypto-pk: s-expression conversions for private and public keys (Dh, Dsa, Rsa) have been removed. You can use PKCS8 for encoding and decoding `X509.{Private,Public}_key.{en,de}code_{der,pem}` (mirage/mirage-crypto#208 @hannesm) * mirage-crypto-pk: in the API, Cstruct.t is no longer present. Instead, string is used (mirage/mirage-crypto#211 @reynir @hannesm) * mirage-crypto-rng: the API uses string instead of Cstruct.t. A new function `generate_into : ?g -> bytes -> ?off:int -> int -> unit` is provided (mirage/mirage-crypto#212 @hannesm @reynir) * mirage-crypto-ec: remove NIST P224 support (mirage/mirage-crypto#209 @hannesm @Firobe) * mirage-crypto: in Uncommon.xor_into the arguments ~src_off and ~dst_off are required now (mirage/mirage-crypto#232 @hannesm), renamed to unsafe_xor_into (98f01b14f5ebf98ba0e7e9c2ba97ec518f90fddc) * mirage-crypto-pk, mirage-crypto-rng: remove type alias "type bits = int" (mirage/mirage-crypto#236 @hannesm) ### Bugfixes * mirage-crypto (32 bit systems): CCM with long adata (mirage/mirage-crypto#207 @reynir) * mirage-crypto-ec: fix K_gen for bitlen mod 8 != 0 (reported in mirage/mirage-crypto#105 that P521 test vectors don't pass, re-reported mirage/mirage-crypto#228, fixed mirage/mirage-crypto#230 @Firobe) * mirage-crypto-ec: zero out bytes allocated for Field_element.zero (reported mirleft/ocaml-x509#167, fixed mirage/mirage-crypto#226 @dinosaure) ### Data race free * mirage-crypto (3DES): avoid global state in key derivation (mirage/mirage-crypto#223 @hannesm) * mirage-crypto-rng: use atomic instead of reference to be domain-safe (mirage/mirage-crypto#221 @dinosaure @reynir @hannesm) * mirage-crypto, mirage-crypto-rng, mirage-crypto-pk, mirage-crypto-ec: avoid global buffers, use freshly allocated strings/bytes instead, avoids data races (mirage/mirage-crypto#186 mirage/mirage-crypto#219 @dinosaure @reynir @hannesm) ### Other changes * mirage-crypto: add {de,en}crypt_into functions (and unsafe variants) to allow less buffer allocations (mirage/mirage-crypto#231 @hannesm) * mirage-crypto-rng-miou: new package which adds rng support with miou (mirage/mirage-crypto#227 @dinosaure) * PERFORMANCE mirage-crypto: ChaCha20/Poly1305 use string instead of Cstruct.t, ChaCha20 interface unchanged, performance improvement roughly 2x (mirage/mirage-crypto#203 @hannesm @reynir) * mirage-crypto-ec, mirage-crypto-pk, mirage-crypto-rng: use digestif for hashes (mirage/mirage-crypto#212 mirage/mirage-crypto#215 @reynir @hannesm) * mirage-crypto-rng: use a set for entropy sources instead of a list (mirage/mirage-crypto#218 @hannesm) * mirage-crypto-rng-mirage: provide a module type S (for use instead of mirage-random in mirage) (mirage/mirage-crypto#234 @hannesm)

@reynir

What's Changed - Fix mixup of subject and hash in error message by @reynir in mirleft/ocaml-x509#165 - Use string instead of cstruct by @dinosaure in mirleft/ocaml-x509#167 - Rename Authenticator.server_{cert,key}_fingerprint by @reynir in mirleft/ocaml-x509#164 - Add Certificate.fold_decode_pem_multiple by @art-w in mirleft/ocaml-x509#169 New Contributors - @art-w made their first contribution in mirleft/ocaml-x509#169

dinosaure and others added 4 commits March 21, 2024 19:55

Remove P224 - see mirage/mirage-crypto#209

26de99b

Remove cstruct from x509 package

bcbf02f

fixes, tests are now passing

bcb9f13

x509 requires ohex

16ede41

dinosaure marked this pull request as ready for review April 4, 2024 14:20

dinosaure and others added 3 commits April 4, 2024 17:26

Update pin-depends (pbkdf) to fix the CI

3dc8b1e

x509 requires OCaml 4.13

099a7ee

ecdsa signature encoding: potentially remove superfluous leading 0s

93d2674

adapt to recent mirage-crypto API change

dd49771

dinosaure mentioned this pull request Apr 20, 2024

Set a new bytes used for elliptic curves computations to '\000' mirage/mirage-crypto#226

Merged

use updated mirage-crypto

61ec7d7

hannesm force-pushed the string branch from 67a37a2 to 61ec7d7 Compare April 23, 2024 15:20

hannesm added 2 commits April 23, 2024 16:31

lower asn1 bound is 0.3.0 now

cbe9b61

lower ohex bound

22bc5b4

hannesm added 3 commits April 23, 2024 18:12

use mirage-crypto at main (since the merge was done)

ba89cbf

algorithm: use unsigned integer, as provided by asn1-combinators 0.3.1

087c021

serial is an unsigned integer now, with a limit of 20 for the octets

67562f7

minor changes for mirage-crypto at HEAD

68e7c8a

hannesm mentioned this pull request Jun 29, 2024

[new release] mirage-crypto (9 packages) (1.0.0) ocaml/opam-repository#26163

Merged